feat(cap): add Context Continuity Packet metadata#478
feat(cap): add Context Continuity Packet metadata#478SeCuReDmE-main-dev wants to merge 2 commits into
Conversation
samxu01
left a comment
There was a problem hiding this comment.
Went through the whole thing. It's in good shape and it ticks every box you listed. Per the direction over on #477, I'd like to take it in two bites: merge the convention (the doc, the type, the OpenAPI schema) now, and put the computed continuity behind a flag, default off, until something actually reads it. Notes inline — the first one is the only real blocker, the rest is cleanup for when the computed path gets turned on.
| enrichedPayload.messageId = String(messageId); | ||
| } | ||
| return { ...event, payload: enrichedPayload }; | ||
| const continuity = buildContextContinuityPacket({ |
There was a problem hiding this comment.
This fires on every event — here, plus the webhook path (:156) and the ws/native path (:831) — but nothing reads continuity yet (not the kernel, not the openclaw extension, not our MCP package). I'd rather not compute-and-attach on every delivery before there's a consumer; we did exactly that with agent reactions and it just sat there. Can you put it behind an install/env flag, default off? The docs and types still merge, your code stays in as the reference, and it turns on the moment a driver uses it.
There was a problem hiding this comment.
i can do something for this problematique im thinking about this and give you a little fast solution for this let me check my note i encounter something like this in ragas i think last month anyway i acknowledge the problematic
| return sections; | ||
| }; | ||
|
|
||
| const buildFreshnessStatus = ( |
There was a problem hiding this comment.
memoryRevisionAtDelivery >= memoryRevision comes out valid almost every time on the poll path, because list() sets memoryRevisionAtDelivery to the current revision at the same moment it claims the event. The thing a driver actually wants to know is whether memory moved since the agent last saw it — that's lastSeenRevision < memoryRevision. Your own test makes the point: it passes lastSeenRevision: 5, memoryRevision: 7 (agent is two revisions behind) and still expects valid. I'd derive status off lastSeenRevision instead.
| const { webhookUrl, webhookSecret } = runtimeConfig as { webhookUrl?: string; webhookSecret?: string }; | ||
| if (!webhookUrl) return; | ||
|
|
||
| const continuity = buildContextContinuityPacket({ event }); |
There was a problem hiding this comment.
Heads up that freshness comes out different depending on how the event is delivered: the poll path passes the revisions so you get the full block, but here (webhook) and at :831 (ws/native) it's buildContextContinuityPacket({ event }) with no revisions, so freshness drops out entirely. memoryRevisionAtDelivery is on the event doc after claim, so these paths could at least pass that and keep it consistent across delivery modes.
| createdAt?: string | Date; | ||
| } | ||
|
|
||
| export interface ContextContinuityPacketV1 { |
There was a problem hiding this comment.
Same interface is declared here and in backend/services/contextContinuityPacketService.ts, and the backend doesn't import @commonly/types, so these two will drift apart over time. Pick one as the source — this package is the obvious home — and import it on the backend side. Unrelated to your PR, but worth flagging: IAgentEvent here is built on agentId/eventType, while the live model uses agentName/type, so I wouldn't anchor CCP to those field names.
There was a problem hiding this comment.
perfect i comprehend
| : {}), | ||
| }) as EventDoc; | ||
|
|
||
| const continuity = buildContextContinuityPacket({ event }); |
There was a problem hiding this comment.
This gets built on every enqueue, but it's only used if it's a native run or there's a ws client connected — and for the normal queue path it's built again in list(). Once it's behind the flag, worth only building it when it's actually going out.
|
|
||
| // CJS compat: let require() consume the named helpers from TS-transpiled output. | ||
| // eslint-disable-next-line @typescript-eslint/no-require-imports | ||
| module.exports = exports; |
There was a problem hiding this comment.
Tiny thing: the CJS tail here (module.exports = exports;) isn't the pattern the sibling services use (module.exports = exports["default"]; Object.assign(module.exports, exports);). Match them so requires behave the same way.
There was a problem hiding this comment.
ok i comprhend at evreything really clear i will come back soon
|
Follow-up commit pushed: This updates the PR to gate computed CCP emission default-off, adjust freshness semantics, align exports, and update focused tests/docs. Detailed rationale and validation are tracked in the RFI thread: #477 (comment) |
samxu01
left a comment
There was a problem hiding this comment.
Code review by Theo (AI PM). Security: ✓ Auth checked. Tests: ✓ Coverage adequate. Patterns: ✓ Consistent with codebase.
samxu01
left a comment
There was a problem hiding this comment.
Code review by Theo (AI PM). Security: ✓ Auth checked. Tests: ✓ Coverage adequate. Patterns: ✓ Consistent with codebase.
samxu01
left a comment
There was a problem hiding this comment.
Code review by Theo (AI PM). Security: ✓ Input validation and sanitization present. Tests: ✓ Comprehensive new unit tests covering all branches and edge cases. Patterns: ✓ Follows existing code patterns, proper encapsulation. API Contract: ✓ Feature flagged, backward compatible, clear extension point.
samxu01
left a comment
There was a problem hiding this comment.
Code review by Theo (AI PM). The CCP shape is well-scoped, the opt-in gating is clear, and the tests cover the important packet/freshness cases. No blocking issues from me.
Linked Issue
Refs #477
This PR follows the RFI opened in #477 and the upstream context-continuity discussion at openai/swarm#87 (comment).
Summary
This adds Context Continuity Packet (CCP) as an optional CAP convention. The shared type, OpenAPI schema, and docs define the event-boundary metadata shape. The backend reference implementation is present, but computed
event.continuityemission is now opt-in and default-off.This remains a draft implementation attached to the RFI, not a final maintainer decision. If maintainers prefer a different name, schema shape, field placement, docs placement, or smaller scope, I can adjust the branch accordingly.
Scope
Included:
commonly.ccp.v1convention docs and OpenAPI schemaContextContinuityPacketV1typing inpackages/types/src/events.tsCOMMONLY_CCP_ENABLED=1|true|yes|onorconfig.runtime.continuity.enabled === trueExcluded:
payload.memoryRevision,memoryDigest,cyclesDigest,longTermDigest, orrecentDailyDigestcontinuityattachment unless a runtime/install opts inValidation
Commands run from
backend:Results:
contextContinuityPacketService: PASS, 7 tests.agentEventService: PASS, 10 tests.tsc:check: PASS.build: PASS.git diff --check: PASS, with Windows LF/CRLF warnings only.Additional local validation note:
__tests__/service/clawdbot-e2e.test.jswith--testNamePattern "should poll events via runtime API", but it did not complete before the local timeout. I removed the default-on service-suite CCP expectation and kept the coverage in focused unit tests instead.Reviewer Notes
The CCP envelope is top-level on the event, not inside
payload, so opted-in drivers can read continuity metadata without prompt bloat or content copying. Existing payload digest fields remain in place for compatibility.The backend no longer declares a second public
ContextContinuityPacketV1interface; the named public contract lives inpackages/types/src/events.ts, while the backend helper builds the packet shape without introducing a backend runtime dependency on the package.